Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Integrate Tanzu Sources (vSphere/Horizon) for Knative #1075

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

lamw
Copy link
Contributor

@lamw lamw commented Jun 27, 2023

Summary

This change refactors VEBA to now use Tanzu Sources for Knative when configuring either vSphere or Horizon sources and maintains event-router for Webhook processor configurations.

In addition, there were few optimizations that were needed including updating the VEBA VM hardware configuration which had some intermittent build errors preventing consistent builds.

Pull Request Checklist

🚨 Please review the guidelines for contributing to this repository.

  • Please ensure that you are making a pull request against the Development branch
  • Please use the WIP keyword in the title of your PR if you are not ready for review
  • Please ensure that you have opened a Github Issue if you are resolving/fixing a problem
  • Please ensure that you have signed all commits and that you have squashed all relevant commits related to your change
  • Please make sure that you have tested your change locally by successfully building and deploying the VMware Event Broker Appliance and/or building and deploying VMware Event Router
  • Please include any relevant screenshots and/or output as part of your testing
  • Please include any documentation updates that is applicable for your changes

Change Type

What types of changes does your code introduce to the VMware Event Broker Appliance?

Put an x in all boxes that apply

Please check the type of change your PR introduces:

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation changes
  • Other (please describe):

Resolved Issues

Closes: #1066

Testing Verification

  • Successfully deployed VEBA appliance using various configurations including: vSphere, vSphere + Webhook, vSphere + Horizion + Webhook Processor

Additional Information

  • Any other details you wish to include or mention

If you have any questions/comments, feel free to reach out to team on Slack #vcenter-event-broker-appliance

Thank you from the VEBA Team! 🥳

@lamw lamw added the enhancement New feature or request label Jun 27, 2023
@lamw lamw added this to the v0.8.0 milestone Jun 27, 2023
@lamw lamw requested a review from embano1 June 27, 2023 18:51
@lamw lamw changed the title WIP feat: Integrate Tanzu Sources (vSphere/Horizon) for Knative feat: Integrate Tanzu Sources (vSphere/Horizon) for Knative Jun 27, 2023
name: default
namespace: vmware-functions
address: #@ "https://"+horizon
#@ if horizonTls == "True":
Copy link
Collaborator

@embano1 embano1 Jun 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to double-check: if horizonTls == "True" means disable TLS or enable TLS? Current wording is confusing in conjunction with skipTLSVerify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, I most likely kept the variable name "simple" when working on vSphere sources from event-router, so just dup'ed variable name here but it is indeed reading from https://github.com/vmware-samples/vcenter-event-broker-appliance/blob/development/files/setup.sh#L34

Happy to modify for clarity purposes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, would be great to have those two align, i.e. if horizonTls == "True" skipTLSVerify should be false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved

echo -e "\e[92mCreating vSphere Secret ..." > /dev/console
# Create vSphere Secret
if [ ${VCENTER_DISABLE_TLS} == "True" ]; then
kn vsphere auth create \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit weird that you deploy most of it using YAML/ytt and here use a CLI. Why not use manifests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started out w/vSphere Sources which provided kn plugin ... I guess looking at Horizon Sources, I had no choice but perhaps might be good to just keep it consistent. I wasn't sure if there was a preference to leverage the plugin over manifests

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no benefit I guess in using the CLI, so let's go for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved

@@ -13,6 +13,11 @@ VEBA_CONFIG_FILE=/root/config/veba-config.json
TINYWWW_TEMPLATE=/root/config/tinywww/templates/tinywww-template.yaml
TINYWWW_CONFIG=/root/config/tinywww/tinywww.yaml

# Basic Auth for TinyWWW endpoints
kubectl -n vmware-system create secret generic basic-auth \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remind me, why and where is this needed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To access any of the endpoints served by TinyWWW - /events, /bootstrap, etc.

Basically, we don't want someone to be able to view events or debug logs w/o some sort of auth

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. And this is not handled by an Ingress? I thought we had those endpoints exposed through ingress which also has auth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we've always had it like this

if [ ${TANZU_SOURCES_DEBUG} -eq "True" ]; then
kubectl -n vmware-sources get cm config-logging -o yaml > /tmp/vmware-sources-config-logging.yaml
sed -i 's/"level": "info"/"level": "debug"/g' /tmp/vmware-sources-config-logging.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used/applied anyhwere? Not sure I'm understanding what this is supposed to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦅 👁️ No, this was temp way of enabling debugging w/sources. Thanks to Gaberiel, this is now part of YTT setup for Sources

Let me remove

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, ok. 🦅 👁️ can still do it 🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved

@lamw lamw force-pushed the issue-1066 branch 2 times, most recently from e3dc440 to 8aa1ca7 Compare July 9, 2023 14:40
Copy link
Collaborator

@embano1 embano1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lamw lamw merged commit 9ba9cdc into vmware-samples:development Jul 10, 2023
@lamw lamw deleted the issue-1066 branch July 10, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Integrate VMware Tanzu Sources for Knative with VEBA
2 participants